bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686

8vasu · 2020-12-08T01:05:23Z

This follows #23536. Also, see #23546, #23740.

tcgetwinsize() and tcsetwinsize() are expected to appear in IEEE Std 1003.1 ("POSIX.1") issue 8 [ the upcoming version of POSIX ] as declarations in <termios.h>; see https://www.austingroupbugs.net/view.php?id=1151#c3856.

NetBSD already has them (thanks to user "kre"):

Update: musl libc also has these functions.
Update2: The patches that I had submitted to add tcgetwinsize() and tcsetwinsize() to the FreeBSD libc have been merged. I am still working on getting them included in glibc.

When POSIX.1 issue 8 is released, termios.tcgetwinsize(), termios.tcsetwinsize() can be updated to utilize the native versions.

Post #23686, #23546, #23740 goals:

add test_winsize() to "Lib/test/test_pty.py";
major revision of "Lib/pty.py", which has not happened since the beginning of the millennium.

Signed-off-by: Soumendra Ganguly soumendraganguly@gmail.com

https://bugs.python.org/issue41818

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu · 2020-12-08T01:43:02Z

@ethanfurman @aeros @gpshead @asvetlov

Update1: These functions perform ioctl()+TIOCGWINSZ/TIOCSWINSZ when the constants are available; they throw NotImplementedError when they are not available. I have implemented os.login_tty(); please see #23740.

Update2: Added ioctl()+TIOCGSIZE to termios.tcgetwinsize().

Update3: Added ioctl()+TIOCSSIZE to termios.tcsetwinsize().

Update4: minor change: unlike struct winsize, the SunOS struct ttysize has members of type int; please refer to https://sourceware.org/git/?p=glibc.git;a=blob;f=bits/ioctl-types.h;h=474a52b4358bc1b4fd211d43ac96f03b168833b7;hb=HEAD#l107

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu · 2020-12-14T01:02:04Z

@zoulasc Sir, can you please take a look at the changes that I made to termios.c? termios_tcgetwinsize_impl() and termios_tcsetwinsize_impl() are the functions that I mentioned earlier.

Modules/termios.c

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

zoulasc

LGTM

8vasu · 2020-12-14T20:39:42Z

@zoulasc Thank you for your time.

…etwinsize(). Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

github-actions · 2021-01-26T00:30:44Z

This PR is stale because it has been open for 30 days with no activity.

8vasu · 2021-01-27T06:50:09Z

Still waiting for core review.

github-actions · 2021-02-28T00:26:51Z

This PR is stale because it has been open for 30 days with no activity.

gpshead

I'd ordinarily also ask for unittest coverage... but this is termios which doesn't appear to have any and might be challenging to automate given its purpose is more for interactive things (?). if you have ideas on that front, feel free to add the beginnings of a Lib/test/test_termios.py - but none of us are going to block this PR on that happening. :)

some comments to address below. primarily: use two item tuples instead of lists.

gpshead · 2021-03-15T00:24:08Z

Modules/termios.c

+ }
+
+ PyObject *v;
+ if (!(v = PyList_New(2))) {


When returning a constant number of values, it is more typical to use a tuple than a list. PyTuple_New and equivalent item setting calls. also update the docstrings in the comment above and on the next function to mention "two item tuple" and use ().

@gpshead Sir, thank you for the review. I should mention that I used PyList_New() instead of PyTuple_New() because I wanted this function to resemble the other low level function termios.tcgetattr(), which also returns a list. Do you confirm this change?

Added: I should have been more specific: termios.tcgetattr() also returns a list of fixed length, and that cannot (?) be changed (backward compatibility?). I was wondering if termios.tcgetwinsize() should also behave similarly.

I ended up making the changes!

Modules/termios.c

bedevere-bot · 2021-03-15T00:32:47Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

8vasu · 2021-08-12T11:42:19Z

@gpshead Unfortunately, I am very busy with my PhD math research currently, and I apologize in advance for not having the time to write unittests for termios :(

Fortunately, I had some time to spare during last December, when I had already written unittests for the pty module, and they were merged (#22962, #23536, #23526). The final goal of this PR (and of #23740 , #23546) is to enhance the pty module, the code for which I already have here https://github.com/8vasu/pypty2, and will create a pull request if/when all of the currently open ones are merged so that all the problems in pty are fixed in a systematic fashion :)

@ambv please take a look at https://bugs.python.org/issue41818 if you have time :)

…t two-item tuples instead of lists.

8vasu · 2021-08-14T06:37:57Z

I have made the requested changes; please review again. Thank you for your patience.

bedevere-bot · 2021-08-14T06:38:00Z

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

…quence, with overflow checking.

Modules/termios.c

gpshead · 2021-08-27T02:57:05Z

Doc/library/termios.rst

@@ -74,6 +74,22 @@ The module defines the following functions:
 output, :const:`TCIOFF` to suspend input, or :const:`TCION` to restart input.


+.. function:: tcgetwinsize(fd)


one follow up PR as I forgot to fix it up here, these need versionadded tags. I'll take care of it.

@gpshead Thank you for making the improvements!

overlooked in #23686

ambv · 2021-08-27T10:03:48Z

Awesome to see this landed 👏🏻👏🏻👏🏻

Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs.

78aaac7

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

the-knights-who-say-ni added the CLA signed label Dec 8, 2020

bedevere-bot added the awaiting review label Dec 8, 2020

Add news, clinic generated code.

0553063

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu force-pushed the tty_winsize branch from a2824f9 to 0553063 Compare December 8, 2020 01:15

8vasu mentioned this pull request Dec 8, 2020

bpo-41818: Make additions to the tty module #23546

Closed

Improve termios.tcsetwinsize().

0d2d35b

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

8vasu mentioned this pull request Dec 11, 2020

bpo-41818: Add os.login_tty() for *nix. #23740

Closed

zoulasc reviewed Dec 14, 2020

View reviewed changes

Modules/termios.c Show resolved Hide resolved

8vasu added 2 commits December 13, 2020 22:15

Add TIOCGSIZE support to termios.tcgetwinsize()

628b7dc

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

Add TIOCSSIZE support to termios.tcsetwinsize()

6ef8a29

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

zoulasc approved these changes Dec 14, 2020

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Dec 14, 2020

The SunOS struct ttysize had fields of type "int"; update termios.tcg…

91374ea

…etwinsize(). Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>

github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 26, 2021

github-actions bot removed the stale Stale PR or inactive for long period of time. label Jan 28, 2021

github-actions bot added the stale Stale PR or inactive for long period of time. label Feb 28, 2021

gpshead requested changes Mar 15, 2021

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Mar 15, 2021

gpshead self-assigned this Mar 15, 2021

gpshead added the type-feature A feature request or enhancement label Mar 15, 2021

termios.tcgetwinsize() and termios.tcsetwinsize() should return/accep…

bb0b2b1

…t two-item tuples instead of lists.

bedevere-bot added awaiting change review and removed awaiting changes labels Aug 14, 2021

bedevere-bot requested a review from gpshead August 14, 2021 06:38

Refactor tcsetwinsize to share common code and accept any two item se…

eb0be09

…quence, with overflow checking.

gpshead reviewed Aug 27, 2021

View reviewed changes

Modules/termios.c Outdated Show resolved Hide resolved

parenthesis on my if statements :)

5ab0d4c

gpshead approved these changes Aug 27, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Aug 27, 2021

gpshead removed the stale Stale PR or inactive for long period of time. label Aug 27, 2021

gpshead approved these changes Aug 27, 2021

View reviewed changes

gpshead merged commit ae224bb into python:main Aug 27, 2021

bedevere-bot removed the awaiting merge label Aug 27, 2021

gpshead reviewed Aug 27, 2021

View reviewed changes

gpshead added a commit that referenced this pull request Aug 27, 2021

bpo-41818: ++ termios versionadded markers. (GH-27987)

245f1f2

overlooked in #23686

8vasu mentioned this pull request Aug 27, 2021

Update ACKS #27988

Merged

8vasu mentioned this pull request Nov 20, 2021

bpo-41818: Add os.login_tty() for *nix. #29658

Merged

8vasu mannequin mentioned this pull request May 5, 2022

Lib/pty.py major revision #85984

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686

bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686

8vasu commented Dec 8, 2020 •

edited

Loading

8vasu commented Dec 8, 2020 •

edited

Loading

8vasu commented Dec 14, 2020

zoulasc left a comment

8vasu commented Dec 14, 2020

github-actions bot commented Jan 26, 2021

8vasu commented Jan 27, 2021

github-actions bot commented Feb 28, 2021

gpshead left a comment

gpshead Mar 15, 2021

8vasu Aug 12, 2021 •

edited

Loading

8vasu Aug 14, 2021

bedevere-bot commented Mar 15, 2021

8vasu commented Aug 12, 2021

8vasu commented Aug 14, 2021

bedevere-bot commented Aug 14, 2021

gpshead Aug 27, 2021

8vasu Aug 28, 2021

ambv commented Aug 27, 2021

		@@ -74,6 +74,22 @@ The module defines the following functions:
		output, :const:`TCIOFF` to suspend input, or :const:`TCION` to restart input.


		.. function:: tcgetwinsize(fd)

bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686

bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686

Conversation

8vasu commented Dec 8, 2020 • edited Loading

8vasu commented Dec 8, 2020 • edited Loading

8vasu commented Dec 14, 2020

zoulasc left a comment

Choose a reason for hiding this comment

8vasu commented Dec 14, 2020

github-actions bot commented Jan 26, 2021

8vasu commented Jan 27, 2021

github-actions bot commented Feb 28, 2021

gpshead left a comment

Choose a reason for hiding this comment

gpshead Mar 15, 2021

Choose a reason for hiding this comment

8vasu Aug 12, 2021 • edited Loading

Choose a reason for hiding this comment

8vasu Aug 14, 2021

Choose a reason for hiding this comment

bedevere-bot commented Mar 15, 2021

8vasu commented Aug 12, 2021

8vasu commented Aug 14, 2021

bedevere-bot commented Aug 14, 2021

gpshead Aug 27, 2021

Choose a reason for hiding this comment

8vasu Aug 28, 2021

Choose a reason for hiding this comment

ambv commented Aug 27, 2021

8vasu commented Dec 8, 2020 •

edited

Loading

8vasu commented Dec 8, 2020 •

edited

Loading

8vasu Aug 12, 2021 •

edited

Loading